feat(sdk): add support to path addressed requests in sdk - #546
feat(sdk): add support to path addressed requests in sdk#546MCarlomagno wants to merge 9 commits into
Conversation
| @@ -220,39 +221,35 @@ impl PubkyResource { | |||
| /// - Returns [`Error::Request`] if the constructed transport URL is invalid. | |||
| pub fn to_transport_url(&self) -> Result<Url, Error> { | |||
| let rel = self.path.as_str().trim_start_matches('/'); | |||
There was a problem hiding this comment.
This line appears in multiple methods. You didn't really touched this code, but would it make sense to encapsulate or make it somehow reusable?.
| /// Render as `https://_pubky.<owner>/<abs-path>` for transport. | ||
| /// Render as `https://_pubky.<owner>/storage/<owner>/<abs-path>` for transport. | ||
| /// | ||
| /// This converts the addressed resource into the actual homeserver URL used |
There was a problem hiding this comment.
This comment and the implementation confuse me. By the actual homeserver URL I would expect homeserver's host or something. Maybe I miss something in terminology here?..
There was a problem hiding this comment.
yes right, I didn't notice the confusing wording, I will replace by something like
"/// Converts the resource into its canonical transport URL"
| let rel = self.path.as_str().trim_start_matches('/'); | ||
| let https = format!("https://_pubky.{}/{}", self.owner.z32(), rel); | ||
| let owner = self.owner.z32(); | ||
| let https = format!("https://_pubky.{owner}/storage/{owner}/{rel}"); |
There was a problem hiding this comment.
AI claims that session.storage().list("/") will not work anymore, because the server allows to list /pub or /priv. Is it intentional?
| .ok_or_else(|| RequestError::Validation { | ||
| message: "path-addressed storage URL is missing a resource path".to_string(), | ||
| })?; | ||
| let owner = PublicKey::try_from_z32(owner).map_err(|_error| RequestError::Validation { |
There was a problem hiding this comment.
We also should reject URLs like https://_pubky.<A>/storage/<B>/... where A != B.
| let https = format!("https://_pubky.{}/{}", self.owner.z32(), rel); | ||
| let owner = self.owner.z32(); | ||
| let path = self.path.as_root_relative_str(); | ||
| let https = format!("https://_pubky.{owner}/storage/{owner}/{path}"); |
There was a problem hiding this comment.
From AI:
The SDK now has two storage URL formats:
- Old homeservers expect:
https://_pubky.<user>/pub/file.txt - New homeservers support:
https://_pubky.<user>/storage/<user>/pub/file.txt
The high-level SDK methods, such as storage().get(...), first ask the homeserver which format it supports. If it is old, the SDK rewrites the URL to the legacy format. So those methods work correctly.
The problem is that public helpers like to_transport_url() always return the new format. If someone passes that URL to a normal HTTP client—or the public synchronous PubkyHttpClient::request()—the compatibility check never happens. An old homeserver receives /storage/..., does not recognize it, and the request fails.
So, in short:
The SDK knows how to support old servers, but that support only exists in the high-level request path. Publicly generated URLs bypass it.
This only affects callers using raw URLs or the low-level request API. Users of storage().get(), put(), etc. are already covered. The likely fixes are either to keep public URLs in the old compatible format or make the public request API perform the same feature negotiation.
2 participants
Closes #527.
Summary
Updates the Rust and JavaScript SDKs to use the path-addressed storage routes introduced in #536 and #543:
/storage/{owner}/pub/file.txt/pub/file.txt{owner}path segmentpubky-hostpubky-hostresolve_pubky,resolvePubky, andPubkyResource::to_transport_urlnow produce the path-addressed form.PubkyResource::from_transport_urlaccepts both forms, taking the owner from the path for canonical URLs even when the request uses an explicit homeserver authority.Acceptance criteria
/storage/{owner}/....resolve_pubky,resolvePubky, andto_transport_urlreturn path-addressed URLs.pubky-host.pubky-host.pubky-hostwire shape.